Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

feat(Input): add iconPosition property#442

Merged
mnajdova merged 12 commits intomasterfrom
feat/input-icon-position
Nov 8, 2018
Merged

feat(Input): add iconPosition property#442
mnajdova merged 12 commits intomasterfrom
feat/input-icon-position

Conversation

@mnajdova
Copy link
Contributor

@mnajdova mnajdova commented Nov 7, 2018

This PR aims to add iconPosition property on the Input component.

API

iconPosition: 'start' | 'end'

new prop for positioning the icon consistent with the other components. By default it's value is 'end'.

Usage

Example code

<Input icon="search" placeholder="Search..." iconPosition="start" />

Rendered component

image

Rendered HTML

<div class="ui-slot ui-input">
  <input type="text" placeholder="Search..." value="" class="ui-slot" />
  <span class="ui-icon" aria-hidden="true"></span>
</div>

@codecov
Copy link

codecov bot commented Nov 7, 2018

Codecov Report

Merging #442 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #442   +/-   ##
=======================================
  Coverage   88.96%   88.96%           
=======================================
  Files          41       41           
  Lines        1386     1386           
  Branches      177      177           
=======================================
  Hits         1233     1233           
  Misses        149      149           
  Partials        4        4
Impacted Files Coverage Δ
src/components/Input/Input.tsx 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ea9bfb...d8ec114. Read the comment docs.

/>
<ComponentExample
title="Icon position"
description="The icon inside the input can be positioned in the start of the input."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*at the start

iconPosition: string
iconRight: string
iconLeft: string
inputPaddingWithIconOnStart: string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: 'at' would be a better fit here: inputPaddingWithIconAtStart

borderColor: v.inputFocusBorderColor,
borderRadius: v.inputFocusBorderRadius,
},
...(p.iconPosition === 'start' &&
Copy link
Contributor

@kuzhelov kuzhelov Nov 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we could simplify and decouple conditions in the following way:

...
...(p.clearable && { padding: v.inputPaddingWithIconAtEnd, }),
...(p.icon && { padding: p.iconPosition === 'start'
  ? v.inputPaddingWithIconAtStart
  : v.inputPaddingWithIconAtEnd })

Most important benefit of this approach would be decoupling the effects of clearable and iconPosition props for styles, so that if, say, one of these props will be eliminated in future, it would be just a matter of removing the corresponding paragraph

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rearranged styles and changed variables names. Thanks!

@mnajdova mnajdova merged commit fd69a5e into master Nov 8, 2018
@layershifter layershifter deleted the feat/input-icon-position branch November 9, 2018 09:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants